Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Crystal 1.14.0 compilation error undefined constant ConnectionBuilder #21

Merged
merged 2 commits into from
Oct 14, 2024
Merged

Fix Crystal 1.14.0 compilation error undefined constant ConnectionBuilder #21

merged 2 commits into from
Oct 14, 2024

Conversation

lachlan
Copy link
Contributor

@lachlan lachlan commented Oct 11, 2024

Compiling the TDS driver with Crystal v1.14.0 results in the following error:

C:\crystal-tds>crystal spec
Showing last frame. Use --error-trace for full trace.

In src\tds\driver.cr:13:5

 13 | ConnectionBuilder.new(connection_options(params), TDS::Connection::Options.from_uri(uri))
      ^----------------
Error: undefined constant ConnectionBuilder

The ConnectionBuilder class had inadvertently been declared with an extra TDS level in its namespace by mistake:

TDS::Driver::TDS::ConnectionBuilder

Then it was later referred to using TDS::ConnectionBuilder which somehow worked on previous versions of the compiler.

This fix removes the extra TDS level from the ConnectionBuilder namespace, as this was a typo, and changes references to use the fully-qualified name TDS::Driver::ConnectionBuilder.

Also fixes the compilation warning that sleep(Number) is deprecated by replacing the calls with sleep(Time::Span) instead.

@lachlan
Fix undefined constant ConnectionBuilder compilation error
5bdec54 
Compiling the TDS driver with Crystal v1.14.0 results in the
following error:

    C:\crystal-tds>crystal spec
    Showing last frame. Use --error-trace for full trace.

    In src\tds\driver.cr:13:5

     13 | ConnectionBuilder.new(connection_options(params), TDS::Connection::Options.from_uri(uri))
          ^----------------
    Error: undefined constant ConnectionBuilder

The `ConnectionBuilder` class had inadvertently been declared with
an extra `TDS` level in its namespace by mistake:

    TDS::Driver::TDS::ConnectionBuilder

Then it was later referred to using `TDS::ConnectionBuilder` which
somehow worked on previous versions of the compiler.

This fix removes the extra `TDS` level from the `ConnectionBuilder`
namespace, as this was a typo, and changes references to use the
fully-qualified name `TDS::Driver::ConnectionBuilder`.
@lachlan
Fix sleep(Number) deprecation compilation warning
c9a2366 
Compiling the TDS driver with Crystal v1.14.0 results in the
following warning:

    C:\crystal-tds>crystal spec
    In spec\spec_helper.cr:16:7

     16 | sleep(5)
          ^----
    Warning: Deprecated ::sleep. Use `::sleep(Time::Span)` instead

    A total of 1 warnings were found.

The `sleep(Number)` method has been deprecated in Crystal v1.14.0,
refer: crystal-lang/crystal#14962.

This fix changes the use of `sleep(Number)` to `sleep(Time::Span)`
in the `spec_helper.cr` file.
@lachlan
Copy link
Contributor Author

lachlan commented Oct 12, 2024 •
edited
Loading

I did some further research, and I believe it was this pull request in Crystal 1.13.0 that changed the compiler behaviour that resulted in the compilation error undefined constant ConnectionBuilder: crystal-lang/crystal#11208.

Given the original (erroneous) declaration of ConnectionBuilder:

class TDS::Driver
    class TDS::ConnectionBuilder
    end
 end

Prior to 1.13.0 the compiler had a bug that resulted in that declaration creating the types ::TDS::Driver and ::TDS::ConnectionBuilder, so later references to the unadorned ConnectionBuilder within Driver resolved without issue as they were in the same module / namespace.

The above pull request in 1.13.0 fixes this compiler bug, so the above declaration now creates the types ::TDS::Driver and ::TDS::Driver::TDS::ConnectionBuilder, so later references to the unadorned ConnectionBuilder within Driver no longer resolves as the types are no longer in the same module / namespace.

This fix puts the ConnectionBuilder type within the ::TDS::Driver namespace, and also uses a fully-qualified reference to ensure there are no type name clashes when other DB drivers are loaded in the same process with the TDS driver.

@wonderix wonderix merged commit 31b8a5b into wonderix:main Oct 14, 2024
@wonderix
Copy link
Owner

Thank you for your PR.

@lachlan lachlan deleted the fix_compile_error_undefined_constant_connectionbuilder branch October 14, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wonderix wonderix wonderix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lachlan @wonderix